Skip to content

[java][js][rb][py][dotnet] Remove firefox cdp#15200

Merged
pujagani merged 17 commits intoSeleniumHQ:trunkfrom
pujagani:remove-firefox-cdp
Feb 3, 2025
Merged

[java][js][rb][py][dotnet] Remove firefox cdp#15200
pujagani merged 17 commits intoSeleniumHQ:trunkfrom
pujagani:remove-firefox-cdp

Conversation

@pujagani
Copy link
Contributor

@pujagani pujagani commented Jan 31, 2025 •
edited by qodo-code-review bot
Loading

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Motivation and Context

Related to #11736

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Enhancement, Bug fix, Tests

Description

  • Removed Firefox CDP (Chrome DevTools Protocol) support across all bindings.

    • Updated FirefoxDriver implementations in Java, .NET, Python, Ruby, and JavaScript.
    • Removed CDP-related methods, capabilities, and warnings.

  • Transitioned to WebDriver BiDi as the recommended protocol for Firefox.

  • Updated and removed tests related to Firefox CDP.

    • Adjusted test configurations and removed Firefox-specific CDP tests.

Changes walkthrough 📝

Relevant files
Enhancement
7 files
FirefoxDriver.java
Removed Firefox CDP support and related capabilities         
+2/-104 
FirefoxOptions.java
Removed CDP-related capability from Firefox options           
+0/-1     
FirefoxDriver.cs
Removed CDP support and related methods from .NET FirefoxDriver
 +1/-86   
webdriver.js
Removed Firefox CDP support and added error for usage       
+2/-8     
driver.rb
Removed CDP support from Ruby FirefoxDriver                           
+0/-18   
webdriver.py
Removed Firefox CDP support and added error for usage       
+3/-15   
driver.rbs
Removed CDP-related methods from Ruby type signatures       
+0/-6     
Tests
4 files
CdpFacadeTest.java
Removed Firefox-specific CDP test annotations                       
+0/-4     
devtools_test.js
Removed Firefox from CDP-related tests                                     
+15/-18 
devtools_spec.rb
Removed Firefox-specific CDP tests                                             
+9/-28   
devtools_tests.py
Removed Firefox-specific CDP warnings in tests                     
+2/-7     
Configuration changes
1 files
BUILD.bazel
Removed Firefox from CDP test configurations                         
+0/-1     
Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
    •

    @pujagani
    Copy link
    Contributor Author

    pujagani commented Feb 3, 2025

    Related blog post: SeleniumHQ/seleniumhq.github.io#2159

    @pujagani
    Copy link
    Contributor Author

    pujagani commented Feb 3, 2025

    Unable to reproduce the failing tests locally so not sure what is happening here. I do not think it is related to the changes made here.

    @pujagani pujagani marked this pull request as ready for review February 3, 2025 09:03
    @qodo-code-review
    Copy link
    Contributor

    qodo-code-review bot commented Feb 3, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling

    The error message for Firefox CDP removal should include more context about using WebDriver BiDi as the alternative, similar to the previous warning message.

    throw new Error('CDP support for Firefox is removed. Please switch to WebDriver BiDi.')
    Configuration Validation

    The remote.active-protocols preference is still being set to 3 despite CDP being removed. This preference may need to be updated or removed if it was specifically for CDP support.

    // https://fxdx.dev/deprecating-cdp-support-in-firefox-embracing-the-future-with-webdriver-bidi/.
addPreference("remote.active-protocols", 3);

    @qodo-code-review
    Copy link
    Contributor

    qodo-code-review bot commented Feb 3, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Improve error message with documentation link

    The error message for Firefox CDP removal should include more context about
    WebDriver BiDi as the recommended alternative, including a link to
    documentation.

    javascript/node/selenium-webdriver/lib/webdriver.js [1247]

    -throw new Error('CDP support for Firefox is removed. Please switch to WebDriver BiDi.')
+throw new Error('CDP support for Firefox is removed. Please switch to WebDriver BiDi - see https://www.selenium.dev/documentation/webdriver/bidirectional/ for migration guide.')
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: The suggestion improves the error message by adding a helpful documentation link for users to learn about WebDriver BiDi migration, making the transition smoother for developers. However, it's a minor enhancement to an already clear error message.

    		5

    @pujagani pujagani merged commit 29e858f into SeleniumHQ:trunk Feb 3, 2025
    27 of 34 checks passed
    @lprimak lprimak mentioned this pull request Feb 23, 2025
    Closed
    valfirst added a commit to vividus-framework/vividus that referenced this pull request Feb 25, 2025
    @valfirst
    Fix broken unit test
    be487ff 
    Root cause: SeleniumHQ/selenium#15200
    valfirst added a commit to vividus-framework/vividus that referenced this pull request Mar 4, 2025
    @valfirst
    Fix broken unit test
    9f378fc 
    Root cause: SeleniumHQ/selenium#15200
    valfirst added a commit to vividus-framework/vividus that referenced this pull request Mar 5, 2025
    @dependabot @valfirst
    Bump org.seleniumhq.selenium:selenium-bom from 4.28.1 to 4.29.0 (#5733)
    df3c50e 
    * Bump org.seleniumhq.selenium:selenium-bom from 4.28.1 to 4.29.0

Bumps [org.seleniumhq.selenium:selenium-bom](https://github.com/SeleniumHQ/selenium) from 4.28.1 to 4.29.0.
- [Release notes](https://github.com/SeleniumHQ/selenium/releases)
- [Commits](https://github.com/SeleniumHQ/selenium/commits/selenium-4.29.0)

---
updated-dependencies:
- dependency-name: org.seleniumhq.selenium:selenium-bom
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Fix broken unit test

Root cause: SeleniumHQ/selenium#15200

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Valery Yatsynovich <valery_yatsynovich@epam.com>
    sandeepsuryaprasad pushed a commit to sandeepsuryaprasad/selenium that referenced this pull request Mar 23, 2025
    @pujagani @sandeepsuryaprasad
    [java][js][rb][py][dotnet] Remove firefox cdp (SeleniumHQ#15200)
    b5cc794 
    Related to SeleniumHQ#11736
    gryznar pushed a commit to gryznar/selenium that referenced this pull request May 17, 2025
    @pujagani @gryznar
    [java][js][rb][py][dotnet] Remove firefox cdp (SeleniumHQ#15200)
    697cb7d 
    Related to SeleniumHQ#11736
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    No reviews

    Assignees

    No one assigned

    Labels

    Review effort [1-5]: 4

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    1 participant

    @pujagani